Skip to content

fix(examples): allocate geo_z instead of geo_y in 3D readGeometry - #375

Open
andrewwhitecdw wants to merge 2 commits into
NVIDIA:mainfrom
andrewwhitecdw:sweep/bugfix-geo-z-alloc
Open

fix(examples): allocate geo_z instead of geo_y in 3D readGeometry#375
andrewwhitecdw wants to merge 2 commits into
NVIDIA:mainfrom
andrewwhitecdw:sweep/bugfix-geo-z-alloc

Conversation

@andrewwhitecdw

Copy link
Copy Markdown

Summary

In the readGeometry() helper of the C API examples, the 3D branch allocated *geo_y a second time instead of allocating *geo_z. The subsequent fscanf then wrote through the (NULL) *geo_z pointer, causing a segmentation fault whenever 3D geometry input was used.

Root cause

*geo_x = (double *)malloc(n * sizeof(double));
*geo_y = (double *)malloc(n * sizeof(double));

if (dimension == 3)
{
    *geo_y = (double *)malloc(n * sizeof(double));   // leaks geo_y,
                                                      // geo_z never allocated
    for (int i = 0; i < n; i ++)
        if (3 != fscanf(fin, "%lf %lf %lf\n",
                        *geo_x + i, *geo_y + i, *geo_z + i))  // write to NULL

Callers initialize gz = NULL, pass &gz, and later free(gz) — so *geo_z is expected to be allocated here.

Fix

Allocate *geo_z in the 3D branch (one-word change in each of examples/amgx_capi.c and examples/amgx_capi_multi.c).

Testing

Full AMGX build/tests were not run (GPU/CUDA-heavy dependency). Instead, readGeometry() was extracted verbatim into a standalone C harness and compiled with gcc -Wall -Wextra (clean):

  • Fixed version: 3D and 2D geometry files parse correctly, geo_z values verified -> PASS (exit 0).
  • Original (pre-fix) version: segfaults on the 3D input (exit 139, SIGSEGV) — confirming the bug and the fix.

Why existing tests missed it

The example programs are not exercised with 3D geometry input files by any automated test; the 2D path (the common case) works fine because geo_z is unused there.

## Summary

In the `readGeometry()` helper of the C API examples, the 3D branch
allocated `*geo_y` a second time instead of allocating `*geo_z`. The
subsequent `fscanf` then wrote through the (NULL) `*geo_z` pointer,
causing a segmentation fault whenever 3D geometry input was used.

## Root cause

```c
*geo_x = (double *)malloc(n * sizeof(double));
*geo_y = (double *)malloc(n * sizeof(double));

if (dimension == 3)
{
    *geo_y = (double *)malloc(n * sizeof(double));   // leaks geo_y,
                                                      // geo_z never allocated
    for (int i = 0; i < n; i ++)
        if (3 != fscanf(fin, "%lf %lf %lf\n",
                        *geo_x + i, *geo_y + i, *geo_z + i))  // write to NULL
```

Callers initialize `gz = NULL`, pass `&gz`, and later `free(gz)` — so
`*geo_z` is expected to be allocated here.

## Fix

Allocate `*geo_z` in the 3D branch (one-word change in each of
`examples/amgx_capi.c` and `examples/amgx_capi_multi.c`).

## Testing

Full AMGX build/tests were not run (GPU/CUDA-heavy dependency).
Instead, `readGeometry()` was extracted verbatim into a standalone C
harness and compiled with `gcc -Wall -Wextra` (clean):

- Fixed version: 3D and 2D geometry files parse correctly, `geo_z`
  values verified -> PASS (exit 0).
- Original (pre-fix) version: segfaults on the 3D input
  (exit 139, SIGSEGV) — confirming the bug and the fix.

## Why existing tests missed it

The example programs are not exercised with 3D geometry input files by
any automated test; the 2D path (the common case) works fine because
`geo_z` is unused there.
Flags the bug where the 3D branch of readGeometry() allocated *geo_y a
second time instead of *geo_z, so fscanf wrote through the NULL *geo_z
pointer and segfaulted on any 3D geometry input file.

The test extracts readGeometry() verbatim from examples/amgx_capi.c and
examples/amgx_capi_multi.c, compiles it with gcc into a small harness,
and runs it on a 3D geometry file.

Red-green verification:
- sh tests/test_geo_z_alloc.sh -> PASS (exit 0) with the fix.
- git show HEAD~1:examples/amgx_capi.c > examples/amgx_capi.c (and same
  for amgx_capi_multi.c), re-run -> FAIL (exit 139, SIGSEGV) on both.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant